feat(configurator): screenshot QA, mobile ergonomics, control UX - #403
Conversation
Looking for bugs?Check back in a few minutes. Qodo's review agents are on it. |
Item 6 — Screenshot QA - Add tests-e2e/screenshots.spec.js: 9 domains × 3 viewports (1280/768/390px) - Add 'screenshots' project to playwright.config.js Item 7 — Mobile ergonomics - app.css: 44px touch targets for cfg-btn/cfg-input/cfg-select at ≤980px - StudioFrame: hide description and shrink h3 at ≤820px - StudioWorkflow + LayoutStudio: horizontal scroll at ≤480px - ControlSection: default-closed on mobile (window.innerWidth < 768) Item 8 — "What does this control do" - FriendlyControl: "drives N" badge via dependentsCount(); ≥10 = master - ControlPreview: caption label below each preview widget - SmartSettings: drives count in section summary; token count in reset Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
21cee20 to
5f21c5f
Compare
|
Warning Review limit reached
More reviews will be available in 50 minutes and 59 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a Chromium-only screenshot project and spec, updates control header metadata and captions, and changes responsive layouts plus touch-target sizing across the configurator. ChangesConfigurator UI updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@configurator/playwright.config.js`:
- Line 37: The default chromium project is still matching screenshots.spec.js,
which causes the same tests to run twice alongside the dedicated screenshots
project. Update the Playwright config so the chromium project excludes
screenshots.spec.js while keeping the separate screenshots project in place;
adjust the project definition(s) in playwright.config.js around the chromium and
screenshots entries so screenshots only run under the screenshots project.
In `@configurator/src/components/ControlSection.svelte`:
- Line 3: The ControlSection initialization reads window.innerWidth directly in
the $state setup, which can crash during SSR. Update ControlSection.svelte so
the initial open state does not access window at module/render time, and move
the viewport-dependent logic into a client-only effect or equivalent guard that
checks typeof window before using innerWidth. Use the existing open state setup
in ControlSection and keep the defaultOpen behavior intact when the client is
available.
In `@configurator/src/styles/app.css`:
- Around line 275-279: The touch-target rules in app.css still allow smaller
controls because `.cfg-btn--sm` and `input[type='range']` override the new 44px
minimum. Update the responsive sizing rules so the relevant selectors
(`.cfg-btn`, `.cfg-btn--sm`, `.cfg-input`, `.cfg-select`, and
`input[type='range']`) all enforce at least 44px in the touch-target media
query, removing or adjusting the smaller overrides so the rule is consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a2829333-db87-41c0-ba92-c9c2b8d5c992
📒 Files selected for processing (10)
configurator/playwright.config.jsconfigurator/src/components/ControlPreview.svelteconfigurator/src/components/ControlSection.svelteconfigurator/src/components/FriendlyControl.svelteconfigurator/src/components/SmartSettings.svelteconfigurator/src/components/editors/LayoutStudio.svelteconfigurator/src/components/editors/StudioFrame.svelteconfigurator/src/components/editors/StudioWorkflow.svelteconfigurator/src/styles/app.cssconfigurator/tests-e2e/screenshots.spec.js
- playwright.config.js: exclude screenshots.spec.js from the default chromium project so it only runs under the dedicated screenshots project (prevents duplicate runs in CI) - app.css: bump .cfg-btn--sm and input[type='range'] to 44px min-height in the touch-target media query so all interactive controls meet WCAG 2.5.5 consistently Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
Summary
Three production-readiness items for the Studio redesign landed in #402.
Changes
Item 6 — Screenshot QA
tests-e2e/screenshots.spec.js: 9 domains × 3 viewports (1280 / 768 / 390 px) — 27 PNGs vianpx playwright test --project=screenshotsplaywright.config.js: newscreenshotsproject (isolated from the regression suite)Item 7 — Mobile ergonomics
app.css: WCAG 2.5.5 touch targets — 44 px min-height for.cfg-btn/.cfg-input/.cfg-selectat ≤980 px;.cfg-btn--smbumped to 36 pxStudioFrame: description text hidden + h3 shrinks toclamp(18px,4vw,26px)at ≤820 px — left column no longer towers above controls on mobileStudioWorkflow+LayoutStudio: horizontal-scroll fallback at ≤480 px so the 4-step workflow nav doesn't squash on narrow phonesControlSection: starts collapsed on mobile (window.innerWidth < 768) to prevent 4000+ px page walls on domains with many sectionsItem 8 — "What does this control do"
FriendlyControl: "drives N" badge usingdependentsCount(); accent colour when ≥10 dependents (master token differentiation)ControlPreview: short caption below every preview widget (colour / typeface / corner / elevation / timing / leading / text wrap / …)SmartSettings: aggregate drives count shown in each section summary; reset button now shows the token count (Reset section (12))Test plan
npm run build— clean build, no Svelte warningsnpm run test:unit— 525 unit tests passnpx playwright test --project=screenshots— 27 PNGs captured, all 27 pass--sf-space-scale) — "drives 45" badge visible in FriendlyControlGenerated by Claude Code
Summary by CodeRabbit